Skip to content

Update and fix SYCL headers #20

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Conversation

ianayl
Copy link
Contributor

@ianayl ianayl commented Aug 19, 2024

I noticed that SYCL headers were outdated and were failing to build when following build instructions in the README. This PR:

  1. Adds the necessary includes in cmake files for builds with -DBUILD_SYCL=on to compile successfully
  2. Removes obsolete references to sycl::info::device::opencl_c_version, which is no longer in the SYCL spec
  3. Changes to use new <sycl/sycl.hpp> headers instead of obsolete <cl/sycl.hpp>

I would like to raise attention to point 2 however: I am unsure if printing the opencl_c_version is important, but judging by the fact that the SYCL spec does not provide alternatives for this in newer specs, I am assuming this is no longer applicable. If the e.g. opencl version is still important debugging information, please let me know and I will add a print for e.g. sycl::info::device::version instead. Thanks for understanding!

Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you check that a few of these changes are required? I seem to be able to build fine without them (tested with the 2024.2 SYCL compiler on Linux). Thanks!

@ianayl
Copy link
Contributor Author

ianayl commented Sep 9, 2024

Could you check that a few of these changes are required? I seem to be able to build fine without them (tested with the 2024.2 SYCL compiler on Linux). Thanks!

I don't actually remember what my environment was that only worked with these changes, but I am unable to replicate that environment now. I've gone ahead and removed the changes, my bad on that part!

Copy link
Contributor

@MichalMrozek MichalMrozek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/ip+1

@ianayl
Copy link
Contributor Author

ianayl commented Feb 6, 2025

Fixes already upstreamed, thanks!

@ianayl ianayl closed this Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants